Skip to content

DOCSP-45972: clarification regarding Database Trigger update endpoint #886

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 9, 2025

Conversation

hunter-mongo
Copy link
Collaborator

Pull Request Info

Jira ticket: https://jira.mongodb.org/browse/DOCSP-45972

Staged changes

Changed description of 'database' param in Database Trigger to clarify that on the shared tier you must include the 'database' parameter or it cannot be saved.

@hunter-mongo hunter-mongo requested a review from dacharyc June 6, 2025 14:14
Copy link

netlify bot commented Jun 6, 2025

Deploy Preview for docs-app-services ready!

Name Link
🔨 Latest commit
🔍 Latest deploy log https://app.netlify.com/projects/docs-app-services/deploys/68472bb4d9b64c1a0ab01828
😎 Deploy Preview https://deploy-preview-886--docs-app-services.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Collaborator

@dacharyc dacharyc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congrats on successfully testing the endpoint and making a PR to this very long, very confusing document! 🎉 I think we can provide better guidance for users who run into this issue, though, so I'd like to see us refine the wording here, please!

@@ -7431,8 +7431,9 @@ components:
description: |-
The name of a database in the linked data source. If you
omit this parameter, the Source Type changes to
"Deployment," unless you are on a shared tier, in which
case App Services will not let you save the trigger.
"Deployment," unless you are on a shared tier. If you are,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, per the ticket, we could be a little more explicit with our guidance here. It might be helpful to actually list the specific error message a user will get if they omit the database param. That way, a user can search for the specific error message and will find a result that explains the issue and how to resolve the problem.

Can you update this to include the error message, and advise shared-tier users to specify the database param to resolve this error?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making this change, @hunter-mongo ! I think we can probably refine the wording further.

First, based on the error message we're documenting, I think the "unless you are on a shared tier" element is not quite accurate. Based on the error message, it seems that even on a shared tier, when you omit the DB param the Source Type still attempts to change to "Deployment." But because shared tier infrastructure does not support deployment changestreams, we reject the change. This may be pedantic, but I would say we could reword this description to something like:

If you omit this parameter, the Source Type changes to "Deployment." However, shared tier infrastructure does not support deployment changestreams. If you omit the database parameter, you receive the deployment changestreams are not supported on shared tier clusters error. Supply the database parameter to resolve this error.

It's more verbose, but I also think it's clearer and more accurate. What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is great and I agree that it is clearer!

Copy link
Collaborator

@dacharyc dacharyc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks for working through this series of changes! Nice work!

Looks like the branch is now out-of-date with the base branch since I already merged my PR. If you need help working through that, let me know!

@hunter-mongo hunter-mongo merged commit 5884ba6 into mongodb:master Jun 9, 2025
8 checks passed
@hunter-mongo hunter-mongo deleted the DOCSP-45972 branch June 9, 2025 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants